fix: spool queued request bodies to break HTTP/2 upload deadlock - #2541
fix: spool queued request bodies to break HTTP/2 upload deadlock#2541dunglas wants to merge 1 commit into
Conversation
Concurrent uploads multiplexed on one HTTP/2 connection hang when there are more streams than PHP threads. A stream queued waiting for a thread keeps its flow-control window open while no one reads the body; enough queued streams exhaust the connection-level window and stall every stream on the connection, including those a thread is already serving. Drain a request body into a buffer (spilling past 2 MiB to a temp file) before it enters the queue, releasing the window so every stream is read by someone. Requests that get a thread immediately still stream live. Bodies overrunning a request_body max_size limit are rejected with 413 instead of reaching PHP truncated. Closes #1074
|
Would it also be possible to just read a fixed amount of bytes ahead and prevent the deadlock like that? (maybe even based on MaxConcurrentStreams) Reading the whole body seems like it might open up other potential issues regarding DOS and requires managing a separate max body size. Also will lead to files being buffered to disk essentially twice. |
|
Thought about this more. Fixed read-ahead doesn't fully solve it: the deadlock is a connection-level window problem, so a small per-stream ration just delays it until a stream stays queued longer than the ration covers. Sizing it correctly needs the negotiated connection window/ To close the DOS gap directly, added two caps:
Both default non-zero: Side note: full-drain-by-default already matches nginx ( WYDT? |
Problem
Concurrent file uploads multiplexed on a single HTTP/2 connection hang indefinitely when there are more streams than PHP threads (#1074).
Root cause is an HTTP/2 flow-control deadlock, not thread-pool exhaustion:
ServeHTTPuntil a PHP thread frees. A queued stream never reads its request body.Body.Readforever. Circular wait.This matches the reproduction: hang at default thread counts, never with separate connections (no multiplexing). With
max_wait_timeunset (infinite), it hangs forever with no error.Fix
Drain a request body into a buffer right before it enters the thread queue, releasing the flow-control window so every stream is read by someone (a thread or the spooler).
spoolRequestBody: buffers to memory, spills past 2 MiB to a temp file, honorsrequest_body_timeoutduring the drain, cleans up the temp file when the request ends.Content-Lengthare spooled; chunked / unknown-length streaming requests keep their live stream.request_body max_sizelimit (http.MaxBytesReader) is rejected with413instead of reaching PHP truncated.Tests
TestConcurrentUploadsHTTP2: 30 concurrent uploads on one h2c connection with 2 threads. Deadlocks without the fix (verified: hangs to the timeout), passes with it.Notes / trade-offs
os.TempDir(), bounded by the client'sContent-Length. Pair withrequest_body max_sizeto cap it.Closes #1074